Implement onboarding, update management, and version 2.1.0 enhancements#39
Implement onboarding, update management, and version 2.1.0 enhancements#39vordenken wants to merge 44 commits into
Conversation
Update version to 2.1.0 in manifest and project files Expose update controller in AppDelegate for onboarding Enable automatic update checks in Info.plist Implement onboarding pages for user guidance Enhance main view with update settings and controls Refactor update controller to manage beta updates Handle onboarding completion and settings in ViewController Create build and release documentation Establish security policy for vulnerability reporting Update appcast for new versioning scheme Add Renovate configuration for dependency management Set version to 2.1.0 in semver file
Reviewer's GuideImplements a three-step onboarding flow with persisted state, exposes and wires update/beta-channel settings through a new UpdateController API into the WebView UI, introduces a GitHub Actions-based automated build-and-release pipeline around semver.txt and Sparkle, and fixes the keyboard PiP toggle to use the W3C PiP API to avoid CSS-sized PiP windows. Sequence diagram for onboarding flow and update settings persistencesequenceDiagram
actor User
participant WebViewHTMLJS as WebView_HTML_JS
participant WKWebView
participant ViewController
participant UpdateController
participant SPUUpdater
participant SFSafariApplication
User->>WKWebView: Launch AutoPiP app
WKWebView->>ViewController: webView(_:didFinish:)
ViewController->>WKWebView: evaluateJavaScript("setVersion")
ViewController->>WKWebView: evaluateJavaScript("startOnboarding()")
User->>WebViewHTMLJS: Click "Done – Open Safari"
WebViewHTMLJS->>WebViewHTMLJS: Collect onb-auto-check/onb-auto-download/onb-beta
WebViewHTMLJS->>WKWebView: postMessage("onboarding-done:" + JSON)
WKWebView->>ViewController: userContentController(_:didReceive:)
ViewController->>ViewController: handleOnboardingDone(jsonString)
ViewController->>UpdateController: set automaticallyChecksForUpdates
ViewController->>UpdateController: set automaticallyDownloadsUpdates
ViewController->>UpdateController: set isBetaUpdatesEnabled
UpdateController->>SPUUpdater: automaticallyChecksForUpdates / automaticallyDownloadsUpdates
UpdateController->>SPUUpdater: resetUpdateCycleAfterShortDelay()
ViewController->>ViewController: UserDefaults["OnboardingCompleted"] = true
ViewController->>SFSafariApplication: showPreferencesForExtension
SFSafariApplication-->>ViewController: completion(error?)
ViewController->>SFSafariApplication: openApplication (fallback if error)
ViewController->>NSApplication: terminate(nil)
Sequence diagram for update and beta channel setting changes in main viewsequenceDiagram
actor User
participant WebViewHTMLJS as WebView_HTML_JS
participant WKWebView
participant ViewController
participant UpdateController
participant UpdaterDelegate
participant SPUUpdater
User->>WKWebView: Reopen AutoPiP app
WKWebView->>ViewController: webView(_:didFinish:)
ViewController->>WKWebView: evaluateJavaScript("setVersion")
ViewController->>ViewController: showMainView()
ViewController->>WKWebView: evaluateJavaScript("show(enabled,useSettings)")
ViewController->>UpdateController: read automaticallyChecksForUpdates / automaticallyDownloadsUpdates / isBetaUpdatesEnabled
ViewController->>WKWebView: evaluateJavaScript("setUpdateSettings(settings)")
User->>WebViewHTMLJS: Toggle "Include beta updates"
WebViewHTMLJS->>WKWebView: postMessage("set-beta:true")
WKWebView->>ViewController: userContentController(_:didReceive:)
ViewController->>UpdateController: isBetaUpdatesEnabled = true
UpdateController->>UserDefaults: set(BetaUpdatesEnabled,true)
UpdateController->>SPUUpdater: resetUpdateCycleAfterShortDelay()
SPUUpdater->>UpdaterDelegate: allowedChannels(for:)
UpdaterDelegate-->>SPUUpdater: Set(["beta"]) (when BetaUpdatesEnabled)
Sequence diagram for keyboard PiP toggle using W3C API with WebKit fallbacksequenceDiagram
actor User
participant ContentScript as content_js
participant VideoElement as HTMLVideoElement
participant BrowserPiP as Browser_PiP_System
User->>ContentScript: Press keyboard shortcut (⌥P)
ContentScript->>ContentScript: togglePiP()
alt PiP currently active
ContentScript->>ContentScript: disablePiP()
else No PiP active
ContentScript->>VideoElement: check typeof video.requestPictureInPicture
alt requestPictureInPicture available
ContentScript->>VideoElement: requestPictureInPicture()
VideoElement-->>BrowserPiP: Open PiP at intrinsic size
VideoElement-->>ContentScript: Promise reject on error
ContentScript->>ContentScript: catch(error)
ContentScript->>ContentScript: setWebkitPresentationMode(video,"picture-in-picture")
else Only WebKit API available
ContentScript->>ContentScript: setWebkitPresentationMode(video,"picture-in-picture")
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughAutoPiP 2.1.0 introduces a multi-page onboarding flow with configurable Sparkle update settings (auto-check, auto-download, beta channel), updates the PiP toggle to prefer the W3C ChangesOnboarding + Update Settings Feature
GitHub Actions Release Pipeline
Community Health and Repository Config
Sequence Diagram(s)sequenceDiagram
rect rgba(173, 216, 230, 0.5)
Note over WKWebView,UpdateController: App launch onboarding path
end
participant WKWebView
participant ViewController
participant UpdateController
participant SafariExtensionState
WKWebView->>ViewController: webView(_:didFinish:)
ViewController->>WKWebView: setVersion(v) via JS
alt Onboarding not complete
ViewController->>WKWebView: startOnboarding() via JS
WKWebView->>ViewController: webkit message: onboarding-done JSON
ViewController->>UpdateController: set auto-check, auto-download, beta channel
ViewController->>ViewController: persist onboarding flag to UserDefaults
ViewController->>ViewController: openSafariAndQuit()
else Onboarding complete
ViewController->>SafariExtensionState: getStateOfSafariExtension (async)
SafariExtensionState-->>ViewController: enabled flag
ViewController->>WKWebView: show(enabled) via JS
ViewController->>UpdateController: read settings
ViewController->>WKWebView: setUpdateSettings(settings) via JS
end
WKWebView->>ViewController: webkit message: set-auto-check / set-beta / check-for-updates
ViewController->>UpdateController: mutate properties / checkForUpdates()
sequenceDiagram
rect rgba(144, 238, 144, 0.5)
Note over Workflow,GitHubRelease: CI release pipeline
end
participant Workflow
participant Keychain
participant Xcode
participant SparkleTools
participant appcast as appcast.xml
participant GitHubRelease
Workflow->>Workflow: gating job checks semver.txt changed
Workflow->>Keychain: import P12 into temp keychain
Workflow->>Xcode: patch versions via sed, archive app, create DMG
Workflow->>SparkleTools: sign_update DMG → EdDSA signature
SparkleTools-->>Workflow: edSignature
Workflow->>appcast: insert new item, deduplicate by shortVersionString
Workflow->>GitHubRelease: create release + upload DMG
Workflow->>appcast: git push to branch + main
Workflow->>Keychain: delete temp keychain (always)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- There are several hard-coded UserDefaults keys (
"OnboardingCompleted","BetaUpdatesEnabled", etc.) scattered across ViewController, the updater delegate, and JavaScript; consider centralizing these keys in a single Swift enum/struct (and mirroring them in JS constants) to avoid typos and make future changes safer. - The string-based message protocol between the web view and
ViewController(e.g."set-auto-check:true","open-url:" + href,"onboarding-done:" + JSON) is starting to get complex; you might want to move to a small structured JSON message format withtype/payload` fields to make parsing more robust and easier to extend.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There are several hard-coded UserDefaults keys (`"OnboardingCompleted"`, `"BetaUpdatesEnabled"`, etc.) scattered across ViewController, the updater delegate, and JavaScript; consider centralizing these keys in a single Swift enum/struct (and mirroring them in JS constants) to avoid typos and make future changes safer.
- The string-based message protocol between the web view and `ViewController` (e.g. `"set-auto-check:true"`, `"open-url:" + href`, `"onboarding-done:" + JSON) is starting to get complex; you might want to move to a small structured JSON message format with `type`/`payload` fields to make parsing more robust and easier to extend.
## Individual Comments
### Comment 1
<location path="AutoPiP/Resources/Script.js" line_range="118-120" />
<code_context>
+ webkit.messageHandlers.controller.postMessage("check-for-updates");
+ });
+
+ document.getElementById('auto-check-toggle').addEventListener('change', function() {
+ webkit.messageHandlers.controller.postMessage("set-auto-check:" + this.checked);
+ var dl = document.getElementById('auto-download-toggle');
+ dl.disabled = !this.checked;
+ if (!this.checked) dl.checked = false;
</code_context>
<issue_to_address>
**issue (bug_risk):** Keep native `autoDownload` flag in sync when disabling it via the `auto-check` toggle.
In this handler you disable and uncheck `auto-download-toggle` only in the DOM, but you never send `set-auto-download:false` to the native side as is done in onboarding. That leaves Sparkle’s `automaticallyDownloadsUpdates` still `true` after a user turns off auto-checking.
You can align behavior by also posting the native message when auto-check is turned off:
```js
document.getElementById('auto-check-toggle').addEventListener('change', function() {
webkit.messageHandlers.controller.postMessage("set-auto-check:" + this.checked);
var dl = document.getElementById('auto-download-toggle');
dl.disabled = !this.checked;
if (!this.checked) {
dl.checked = false;
webkit.messageHandlers.controller.postMessage("set-auto-download:false");
}
});
```
</issue_to_address>
### Comment 2
<location path="BUILD.md" line_range="55" />
<code_context>
+### GitHub App for Branch Protection
+
+If `main` has branch protection, the workflow needs a GitHub App to push the appcast update. Create a GitHub App with **Contents: Read & Write** permission, install it on the repo, then:
+- Add variable `CLienT_ID` with the App's client ID
+- Add secret `APP_PRIVATE_KEY` with the App's private key
+- Add the App to the branch protection bypass list
</code_context>
<issue_to_address>
**issue (typo):** Typo in variable name: `CLienT_ID` should be `CLIENT_ID` for consistency
Below you use `CLIENT_ID` in the secrets section. Please update this bullet to `CLIENT_ID` as well so the variable name is consistent and avoids configuration confusion.
```suggestion
- Add variable `CLIENT_ID` with the App's client ID
```
</issue_to_address>
### Comment 3
<location path="BUILD.md" line_range="49" />
<code_context>
+- Sets the version in the Xcode project and `manifest.json`
+- Builds, archives, and creates a DMG
+- Signs the DMG with the code signing certificate and Sparkle EdDSA
+- Creates a git tag and GitHub Release with changelog + installation instructions
+- Updates `appcast.xml` (with changelog as `<description>`) on the working branch and `main`
+
</code_context>
<issue_to_address>
**nitpick (typo):** Consider capitalizing "Git" as a proper noun
Here, "git" should be capitalized as "Git" to match its proper name and stay consistent with "GitHub" in the same sentence.
```suggestion
- Creates a Git tag and GitHub Release with changelog + installation instructions
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
.github/workflows/build-release.yml (1)
18-18: Pin GitHub Actions to commit SHAs for supply-chain security.These actions use tag-pinning (
@v*) instead of immutable commit SHAs, which increases supply-chain attack risk. Consider enabling Renovate'shelpers:pinGitHubActionDigestspreset (or upgrading toconfig:best-practices) to automatically pin actions to full-length commit SHAs with version comments for maintainability.Lines: 18, 68, 74, 341, 400
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-release.yml at line 18, The GitHub Actions in this workflow are pinned to semantic version tags (such as `@v6` in actions/checkout@v6) instead of immutable commit SHAs, which introduces supply-chain security risks. Replace each action version tag reference with the full commit SHA of that action version, and include the version tag as a comment for maintainability (for example, replace `@v6` with @<full-commit-sha> # v6). Alternatively, enable Renovate's helpers:pinGitHubActionDigests preset or upgrade to config:best-practices to automatically manage this pinning going forward.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/ISSUE_TEMPLATE/config.yml:
- Line 1: In the `.github/ISSUE_TEMPLATE/config.yml` file, rename the
configuration key from `blank_issues` to `blank_issues_enabled`. GitHub's issue
template configuration schema uses `blank_issues_enabled` as the correct key
name to control whether blank issues are available in the issue template
chooser. Update this key name to match the expected schema.
In @.github/workflows/build-release.yml:
- Around line 8-10: The workflow-level permissions are granting write access to
all jobs via `contents: write`, which violates the principle of least privilege.
Remove the workflow-level `contents: write` permission to revert to the default
read-only state, then add a job-level permissions section specifically to the
`build-and-release` job that grants `contents: write` only to that job. This
ensures the `check` job and any other jobs retain read-only permissions while
only the `build-and-release` job has the necessary write access.
- Around line 124-130: The tag existence check in the workflow hard-fails if a
tag already exists, which prevents idempotent reruns when a release partially
succeeds but a later step fails. Instead of exiting with an error when the tag
is detected, modify the logic to handle the existing tag case gracefully by
updating or reconciling the existing release and appcast rather than aborting.
This allows subsequent workflow reruns to recover from failures and complete the
release process without being blocked by the previously-created tag.
- Around line 24-33: Refactor the GitHub Actions workflow to prevent template
injection and spoofing vulnerabilities by moving GitHub context expressions from
direct `run:` script expansion to the `env:` section. Replace the bot detection
logic that uses `github.event.head_commit.author.name` (which is user-controlled
and spoofable) with `github.actor` instead. Define environment variables in the
`env:` block above the `run:` step, then reference them as shell variables
within the script. Apply this same hardening pattern at lines 93 and 417 where
similar direct template expansion and bot detection logic exists.
In `@AutoPiP/UpdateController.swift`:
- Around line 30-38: The setters for automaticallyChecksForUpdates and
automaticallyDownloadsUpdates in UpdateController do not enforce the constraint
that auto-download must be gated by auto-check (auto-download cannot be true if
auto-check is false). Update the setter for automaticallyChecksForUpdates to set
automaticallyDownloadsUpdates to false whenever auto-check is disabled, and
update the setter for automaticallyDownloadsUpdates to prevent enabling
auto-download when auto-check is false (either by rejecting the change or
forcing auto-check to true). This ensures the dependency is enforced at the
controller layer and prevents callers from creating invalid states.
In `@BUILD.md`:
- Line 55: Fix the variable name typo in the GitHub App setup documentation at
line 55 of BUILD.md. The variable name is currently written as `CLienT_ID` with
inconsistent capitalization, which will cause workflow configuration to fail.
Correct this to the proper constant naming convention `CLIENT_ID` to match the
actual variable name used in the application configuration.
In `@README.md`:
- Line 40: The update note in the README at the "Updating" section states that
Sparkle automatically checks for updates once per day as unconditional behavior.
Since auto-update checks are now user-configurable via a toggle, revise this
sentence to clarify that automatic daily checks only occur when the user has
enabled this feature. Reference the new toggle in the statement to make it clear
that users can control whether this automatic checking behavior is active.
---
Nitpick comments:
In @.github/workflows/build-release.yml:
- Line 18: The GitHub Actions in this workflow are pinned to semantic version
tags (such as `@v6` in actions/checkout@v6) instead of immutable commit SHAs,
which introduces supply-chain security risks. Replace each action version tag
reference with the full commit SHA of that action version, and include the
version tag as a comment for maintainability (for example, replace `@v6` with
@<full-commit-sha> # v6). Alternatively, enable Renovate's
helpers:pinGitHubActionDigests preset or upgrade to config:best-practices to
automatically manage this pinning going forward.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a48e89bb-c91e-48e8-8bc8-32b3bd951291
📒 Files selected for processing (22)
.github/ISSUE_TEMPLATE/bug_report.yml.github/ISSUE_TEMPLATE/config.yml.github/ISSUE_TEMPLATE/feature_request.yml.github/pull_request_template.md.github/workflows/build-release.ymlAutoPiP Extension/Resources/content.jsAutoPiP Extension/Resources/manifest.jsonAutoPiP.xcodeproj/project.pbxprojAutoPiP.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolvedAutoPiP/AppDelegate.swiftAutoPiP/Info.plistAutoPiP/Resources/Base.lproj/Main.htmlAutoPiP/Resources/Script.jsAutoPiP/Resources/Style.cssAutoPiP/UpdateController.swiftAutoPiP/ViewController.swiftBUILD.mdREADME.mdSECURITY.mdappcast.xmlrenovate.jsonsemver.txt
…hen checks are turned off
…sabled when auto-check is turned off
…s and enhance versioning logic
… paths-ignore comments
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
❌ Failed to create PR with unit tests: AGENT_CHAT: Failed to open pull request |
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The GitHub Actions trigger uses
paths-ignore: ['.github/**']while the comment above says “this workflow itself still trigger a build”; if you want changes to workflow files to trigger builds, remove.github/**frompaths-ignoreor narrow it (e.g. only ignore.github/ISSUE_TEMPLATE/**). - The WKWebView
evaluateJavaScript("setUpdateSettings(...)" )call inshowMainView()runs outside of aDispatchQueue.main.asyncblock, unlike the extension state call; consider dispatching it explicitly to the main thread to avoid any potential threading issues with WKWebView.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The GitHub Actions trigger uses `paths-ignore: ['.github/**']` while the comment above says “this workflow itself still trigger a build”; if you want changes to workflow files to trigger builds, remove `.github/**` from `paths-ignore` or narrow it (e.g. only ignore `.github/ISSUE_TEMPLATE/**`).
- The WKWebView `evaluateJavaScript("setUpdateSettings(...)" )` call in `showMainView()` runs outside of a `DispatchQueue.main.async` block, unlike the extension state call; consider dispatching it explicitly to the main thread to avoid any potential threading issues with WKWebView.
## Individual Comments
### Comment 1
<location path=".github/workflows/build-release.yml" line_range="143-150" />
<code_context>
+ echo '```' >> "$GITHUB_STEP_SUMMARY"
+ fi
+
+ - name: Check for existing tag
+ run: |
+ TAG="${{ steps.version.outputs.tag }}"
+ if git rev-parse "refs/tags/${TAG}" >/dev/null 2>&1; then
+ echo "::warning::Tag ${TAG} already exists — assuming partial release, will reconcile (re-upload asset, refresh appcast)."
+ echo "tag_exists=true" >> "$GITHUB_OUTPUT"
+ else
+ echo "tag_exists=false" >> "$GITHUB_OUTPUT"
+ fi
+
</code_context>
<issue_to_address>
**nitpick:** The `Check for existing tag` step computes an output that is never consumed, and the step has no `id`.
This step writes `tag_exists` to `$GITHUB_OUTPUT`, but without an `id` the output can’t be used and isn’t referenced anywhere else. Either assign an `id` and consume `steps.<id>.outputs.tag_exists` where needed, or remove the unused output to keep the workflow simpler and clearer.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| - name: Check for existing tag | ||
| run: | | ||
| TAG="${{ steps.version.outputs.tag }}" | ||
| if git rev-parse "refs/tags/${TAG}" >/dev/null 2>&1; then | ||
| echo "::warning::Tag ${TAG} already exists — assuming partial release, will reconcile (re-upload asset, refresh appcast)." | ||
| echo "tag_exists=true" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "tag_exists=false" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
nitpick: The Check for existing tag step computes an output that is never consumed, and the step has no id.
This step writes tag_exists to $GITHUB_OUTPUT, but without an id the output can’t be used and isn’t referenced anywhere else. Either assign an id and consume steps.<id>.outputs.tag_exists where needed, or remove the unused output to keep the workflow simpler and clearer.
🚀 Release v2.1.0 — Onboarding & Automated Releases
This PR promotes v2.1.0 from beta to stable after 13 beta releases and extensive testing.
✨ What's New
🎨 Onboarding Flow
First-time users are now guided through a 3-step setup:
State is persisted via
UserDefaults(OnboardingCompletedflag) — the flow only runs once per user.⚙️ Configurable Update Settings
Update preferences are now first-class settings, exposed via the onboarding flow and the main app view:
All settings are wired through
UpdateControllerto the underlying SparkleSPUUpdater.🔧 Under the Hood
🤖 Automated Builds & Releases
Releases are now fully automated via GitHub Actions (
.github/workflows/build-release.yml):semver.txtis the single source of truth for version + changelogfeature/*→ auto-incrementedv2.1.0-beta{N}(never overwritten)mainwith changedsemver.txt→ stablev2.1.0releaseappcast.xml🧪 Beta Release Channel
Users can now opt in to beta updates through Sparkle's channel mechanism (
allowedChannelsreturns["beta"]whenBetaUpdatesEnabledis true). The appcast marks beta releases with a visible warning banner.🐛 Bugfix: Keyboard PiP small in corner
Fixed a bug where the keyboard shortcut (
⌥P) opened PiP at the video element's CSS-rendered size instead of its intrinsic dimensions. The toggle now prefers the W3CrequestPictureInPicture()API and falls back to the WebKit API on failure. This affected YouTube's mini-player (which renders the<video>element very small) and other CSS-scaled video players.🧪 Testing
This release has been validated through 13 beta releases with no regressions reported. Manual test plan:
📝 Notes
appcast.xmlonmainwill be auto-updated by the workflow after merge — the beta banner will be removed and thesparkle:channelwill be set tostable.✅ Checklist
semver.txt,manifest.json, and Xcode projectSummary by Sourcery
Introduce a guided onboarding flow, configurable update channel and settings, and automated build-and-release pipeline for the AutoPiP v2.1.0 release.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Chores:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation